Skip to content

Keep v0 rules named 'if' from fusing into a preceding comp+if rule - #25

Merged
FallenAngel97 merged 2 commits into
FallenAngel97:masterfrom
lovesegfault:fix-if-func-fusion
Jul 11, 2026
Merged

Keep v0 rules named 'if' from fusing into a preceding comp+if rule#25
FallenAngel97 merged 2 commits into
FallenAngel97:masterfrom
lovesegfault:fix-if-func-fusion

Conversation

@lovesegfault

Copy link
Copy Markdown
Contributor

Stacked on #23 — the first commit here is that branch; review commit 2572d30 only. Closes the second "known remaining gap" listed in #23. Sibling of #24 (independent; whichever merges second needs tree-sitter generate re-run for the generated-file overlap).

Problem

A legacy v0 policy defining a function or object rule literally named if directly after a constant rule fused into it:

a := 1

if(x) := x

parsed as one rule — head a := 1 if, body (x) := x — corrupting both rules. Same for if[x] := 2. (if is a reserved word in OPA v1, so only legacy v0 policies define rules named if, but they shouldn't corrupt the preceding statement.)

Approach

Precedence cannot separate the readings: a draft that weighted the function head form above the fused parse broke a := 1 if (x) followed by b := 2 — the same tokens, but there the fusion is correct. What distinguishes the bad fusions structurally is the body shape: a top-level assignment whose left-hand side starts with ( or [ is exactly what a v0 rule named if leaves behind, and is not a plausible body (OPA rejects assigning to a call or array there).

  • The unbraced first body of a comp+if rule now excludes assignment/unification literals with a parenthesized or array-shaped lhs, via restricted twins of the literal/expr/infix rules — all aliased back to the regular node names, so node-types.json is unchanged and consumers see no new node types. Ordinary assignment bodies keep their one-rule parse (a := 1 if x := 2, a := 1 if input.x = 1).
  • The left operand of non-assignment infix recurses through the same restriction: assignment binds tighter than the other operators, so in (x) := x + 1 the + is the top operator and the excluded shape would otherwise hide as its lhs (if(x) := x + 1 fused through that hole).
  • Follow-on bodies of a comp+if rule drop the bare := term value form — the value already lives in the head. Without this the function case re-fused as first body (x), second body := x. Else clauses keep their optional value (else := 2).

Corpus

Eight new cases: the function and object splits with term and expression values under both := and =; the unbraced assignment and unification bodies that must keep fusing; and a parenthesized body followed by a constant (the case that killed the precedence draft). 64/64 pass; src/ regenerated with tree-sitter generate (STATE_COUNT +4%, no large-state growth).

Remaining gap

The same rule-named-if fusion after a value-carrying func/obj/contains head (f(x) := 1 then if(y) := y) predates this change and still fuses — fixing it means routing the restricted bodies through every if-terminated head form, which deserves its own change.

@FallenAngel97

Copy link
Copy Markdown
Owner

Looks great to me 😁
But I cannot resolve conflicts until 10th of July, I am only with mobile phone.

A v0 policy defining a function or object rule literally named 'if'
directly after a constant rule fused into it: 'a := 1' followed by
'if(x) := x' parsed as one rule with head 'a := 1 if' and body
'(x) := x', and 'if[x] := 2' the same way with body '[x] := 2'.
('if' is a reserved word in OPA v1, so only legacy v0 policies hit
this, but they should not corrupt the preceding rule.)

Precedence cannot separate the readings: an early draft that weighted
the function head form above the fused parse broke 'a := 1 if (x)'
followed by 'b := 2' — the same tokens where the fusion is correct.
What distinguishes the bad fusions structurally is the body shape: a
top-level assignment whose left-hand side starts with '(' or '[' is
exactly what a v0 rule named 'if' leaves behind, and is not a
plausible body (OPA rejects assigning to a call or array there).

So the unbraced first body of a comp+if rule now excludes assignment
and unification literals with a parenthesized or array-shaped lhs,
via restricted twins of the literal/expr/infix rules, all aliased
back to the regular node names so consumers see no new node types.
Ordinary assignment bodies keep their one-rule parse ('a := 1 if
x := 2', 'a := 1 if input.x = 1' — the first cut excluded all
top-level assignments and silently split these into a constant plus
a phantom rule named 'if'). The left operand of non-assignment infix
recurses through the same restriction: assignment binds tighter, so
in '(x) := x + 1' the '+' is the top operator and the excluded shape
would otherwise hide as its lhs.

The follow-on bodies of a comp+if rule drop the bare ':= term' value
form (the value already lives in the head) — without this, the
function case re-fused as first body '(x)' plus second body ':= x'.
Else clauses keep their optional value.

Eight new corpus cases pin the function and object splits (term and
expression values, ':=' and '='), the unbraced assignment and
unification bodies that must keep fusing, and a parenthesized body
followed by a constant. 64/64 corpus tests pass; src/ regenerated
with tree-sitter generate.

Remaining gap: the same rule-named-'if' fusion after value-carrying
func/obj/contains heads ('f(x) := 1' then 'if(y) := y') predates this
change and needs the restricted bodies routed through every
if-terminated head form.
The deploy step force-pushes the playground to gh-pages, but on pull
requests from forks GITHUB_TOKEN is read-only, so the push fails with
403 and marks CI red on every fork PR. Only deploy on pushes to master.
@lovesegfault

Copy link
Copy Markdown
Contributor Author

@FallenAngel97 no worries, fixed & fixed ci :)

@FallenAngel97
FallenAngel97 merged commit 7f8a431 into FallenAngel97:master Jul 11, 2026
1 check passed
@FallenAngel97

Copy link
Copy Markdown
Owner

Thanks @lovesegfault !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants